Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

misc(v2): change blog front matter to snake_case #1989

Merged
merged 4 commits into from
Nov 19, 2019
Merged

Conversation

yangshun
Copy link
Contributor

Motivation

Standardize our front matter. Front matter should be snake_case 🐍. I'm preserving the camelCase for legacy reasons, so users using the camelCase will still work.

Also, I'm removing two following fields:

  • authorFBID - This is really unnecessary and we don't need it. People should just provide the FB Graph API image URL if they wanted a FB profile picture
  • authorTwitter- This was never ported to D2, but is still in the template and examples

I'm not updating the docs and template yet because it should be done only after alpha.35 is published or else there will be a mismatch between the doc and latest version.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Blog loads.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@yangshun yangshun requested a review from endiliey as a code owner November 14, 2019 01:13
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 14, 2019
@@ -1,3 +1,10 @@
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in this file are just nits.

@@ -11,7 +18,7 @@ export function truncate(fileString: string, truncateMarker: RegExp | string) {
}

// YYYY-MM-DD-{name}.mdx?
// prefer named capture, but old node version do not support
// prefer named capture, but old Node version does not support.
const FILENAME_PATTERN = /^(\d{4}-\d{1,2}-\d{1,2})-?(.*?).mdx?$/;

function toUrl({date, link}: DateLink) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@endiliey why is there a hardcoded date 2019-01-01 here? On L53 as well o.o

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not me who put that. Iirc its hardcoded that way by some prevs contributor who added cjk support.

He wanted to make it clear that its yyyy-mm-dd and then take the length lol.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just substring(0,9) lol

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Nov 14, 2019

Deploy preview for docusaurus-2 ready!

Built with commit 79760c1

https://deploy-preview-1989--docusaurus-2.netlify.com

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Nov 14, 2019

Deploy preview for docusaurus-preview ready!

Built with commit 79760c1

https://deploy-preview-1989--docusaurus-preview.netlify.com

@endiliey endiliey added the pr: breaking change Existing sites may not build successfully in the new version. Description contains more details. label Nov 14, 2019
Copy link
Contributor

@endiliey endiliey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just add the docs but dont merge. I will merge right before release. I can also freeze netlify prod build if needed

@yangshun
Copy link
Contributor Author

@endiliey done. Thanks for the tip

@endiliey endiliey merged commit 65965e0 into master Nov 19, 2019
@endiliey endiliey deleted the yangshun/blog branch November 19, 2019 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: breaking change Existing sites may not build successfully in the new version. Description contains more details.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants